Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WSTEAMA-1129 Unordered list items in the footer component should not render empty li elements, or have a role="listitem" #12286

Merged
merged 12 commits into from
Jan 20, 2025

Conversation

LilyL0u
Copy link
Contributor

@LilyL0u LilyL0u commented Jan 15, 2025

Resolves JIRA [1129]

Overall changes

The issue was that in the case where we do not have the 'do not sell or share my info' link in the footer (it is on amp and not canonical when inside the UK), then the elements array containing the links to include in the links had one null value in it, and this was being rendered as an empty list item. By filtering out the null value, we no longer have this empty list item in the footer.

Code changes

Updated the elements array mapping logic to filter out null values using .filter(Boolean).

I have also removed role="listitem" as requested in the jira ticket.

Testing

Manually check the footer by inspecting the list of items in the footer and ensuring that on amp and canonical there are no empty list items. Check the layout of the page does not change too much.

The Jira ticket also suggests the following a11y checks:

VoiceOver iOS 17.3.1 or later with Safari
VoiceOver Mac on latest iOS version with Safari
Jaws latest demo version with Chrome
Jaws latest demo version with Edge
NVDA latest version available with FireFox ESR
NVDA latest version available with Chrome

as there have been some updates in screenreader software.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

});
return null;
})
.filter(Boolean);
Copy link
Contributor Author

@LilyL0u LilyL0u Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is the only code change here in this file. The rest is formatting.

Copy link
Contributor

@karinathomasbbc karinathomasbbc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Great to finally have this issue fixed :)

@karinathomasbbc
Copy link
Contributor

Have we addressed the "role="listitem" for footer links" - i.e. remove role list item for all li’s if not needed part of the ticket yet?

@LilyL0u
Copy link
Contributor Author

LilyL0u commented Jan 15, 2025

Have we addressed the "role="listitem" for footer links" - i.e. remove role list item for all li’s if not needed part of the ticket yet?

I can do that too. I realised I misread the comment about it on the jira ticket. Should be easy.

@greenc05
Copy link

greenc05 commented Jan 20, 2025

Thanks @LilyL0u A quick check with our supported screen readers would be good please, to check we still have list semantics after the removal of the role (if you haven't already done this)

@LilyL0u
Copy link
Contributor Author

LilyL0u commented Jan 20, 2025

Thanks @LilyL0u A quick check with our supported screen readers would be good please, to check we still have list semantics after the removal of the role (if you haven't already done this)

@greenc05
@MeriemMechri checked with a screenreader and the remaining items were in a list

image (10)

@LilyL0u LilyL0u merged commit 007f623 into latest Jan 20, 2025
11 checks passed
@LilyL0u LilyL0u deleted the WSTEAMA-1129-ul-list-items-should-not-render-empty branch January 20, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants